Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscellaneous inlining improvements #69256

Merged
merged 3 commits into from
Feb 20, 2020
Merged

Conversation

nnethercote
Copy link
Contributor

These commits inline some hot functions that aren't currently inlined, for some speed wins.

r? @Centril

It only has two call sites, and the one within `from_utf8` is hot within
rustc itself.
Mostly, these are the ones whose body just contains `f(self)`.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 18, 2020
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Feb 18, 2020

⌛ Trying commit e761f3a with merge 2a7a3a1...

bors added a commit that referenced this pull request Feb 18, 2020
Miscellaneous inlining improvements

These commits inline some hot functions that aren't currently inlined, for some speed wins.

r? @Centril
@bors
Copy link
Contributor

bors commented Feb 18, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 18, 2020
@nnethercote
Copy link
Contributor Author

The try push failed due to a disk being full. Let's try again.

@bors try

@bors
Copy link
Contributor

bors commented Feb 18, 2020

⌛ Trying commit e761f3a with merge 995d70a9f461625ffa1dd32e8a1973b9c3ed4872...

@nnethercote
Copy link
Contributor Author

Local check results are good, mostly benefiting short-running benchmarks.

helloworld-check
        avg: -4.4%      min: -5.0%      max: -4.0%
deeply-nested-check
        avg: -2.8%      min: -4.3%      max: -1.9%
issue-46449-check
        avg: -3.4%      min: -4.2%      max: -2.8%
unify-linearly-check
        avg: -3.4%      min: -4.2%      max: -2.4%
trait-stress-check
        avg: -1.5%      min: -4.1%      max: -0.1%
await-call-tree-check
        avg: -3.0%      min: -4.0%      max: -2.2%
wf-projection-stress-65510-che...
        avg: -1.1%      min: -2.9%      max: -0.1%
regression-31157-check
        avg: -1.6%      min: -2.5%      max: -0.8%
tokio-webpush-simple-check
        avg: -1.3%      min: -1.8%      max: -0.8%
token-stream-stress-check
        avg: -1.3%      min: -1.3%      max: -1.2%
webrender-wrench-check
        avg: -0.9%      min: -1.2%      max: -0.5%
clap-rs-check
        avg: -0.6%      min: -1.2%      max: 0.2%
wg-grammar-check
        avg: -0.6%      min: -1.2%      max: -0.3%
ripgrep-check
        avg: -0.8%      min: -1.1%      max: -0.4%
inflate-check
        avg: -0.4%      min: -1.1%      max: -0.1%
futures-check
        avg: -0.7%      min: -1.0%      max: -0.3%
encoding-check
        avg: -0.7%      min: -0.9%      max: -0.4%
coercions-check
        avg: -0.6%?     min: -0.9%?     max: -0.3%?
tuple-stress-check
        avg: -0.2%      min: -0.8%      max: 0.0%
regex-check
        avg: -0.7%      min: -0.8%      max: -0.3%
syn-check
        avg: -0.5%      min: -0.7%      max: -0.3%
unicode_normalization-check
        avg: -0.3%      min: -0.7%      max: -0.1%
piston-image-check
        avg: -0.5%      min: -0.7%      max: -0.2%
hyper-2-check
        avg: -0.4%      min: -0.6%      max: -0.2%
keccak-check
        avg: -0.2%      min: -0.5%      max: -0.1%
webrender-check
        avg: -0.4%      min: -0.5%      max: -0.2%
html5ever-check
        avg: -0.3%      min: -0.5%      max: -0.1%
unused-warnings-check
        avg: -0.3%      min: -0.4%      max: -0.2%
deep-vector-check
        avg: -0.2%      min: -0.4%      max: -0.2%
serde-check
        avg: -0.2%      min: -0.3%      max: -0.1%
cranelift-codegen-check
        avg: -0.2%      min: -0.3%      max: -0.1%
ucd-check
        avg: -0.1%      min: -0.3%      max: 0.1%
cargo-check
        avg: -0.2%      min: -0.3%      max: -0.1%
script-servo-check
        avg: -0.2%      min: -0.3%      max: -0.1%
serde-serde_derive-check
        avg: -0.2%      min: -0.2%      max: -0.1%
style-servo-check
        avg: -0.2%      min: -0.2%      max: -0.1%
ctfe-stress-4-check
        avg: -0.1%?     min: -0.2%?     max: -0.1%?
packed-simd-check
        avg: -0.1%      min: -0.1%      max: -0.0%

@Centril
Copy link
Contributor

Centril commented Feb 18, 2020

r=me with positive perf.rl.o numbers, etc. etc.

@bors
Copy link
Contributor

bors commented Feb 19, 2020

☀️ Try build successful - checks-azure
Build commit: 995d70a9f461625ffa1dd32e8a1973b9c3ed4872 (995d70a9f461625ffa1dd32e8a1973b9c3ed4872)

@rust-timer
Copy link
Collaborator

Queued 995d70a9f461625ffa1dd32e8a1973b9c3ed4872 with parent e620d0f, future comparison URL.

@nnethercote
Copy link
Contributor Author

nnethercote commented Feb 19, 2020

@bors rollup=never, because it affects performance.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 995d70a9f461625ffa1dd32e8a1973b9c3ed4872, comparison URL.

@bjorn3
Copy link
Member

bjorn3 commented Feb 19, 2020

There are many improvements, but syn-opt regressed.

@mati865
Copy link
Contributor

mati865 commented Feb 19, 2020

@bjorn3 could be noise: #69060

@nnethercote
Copy link
Contributor Author

Yes, syn is known to be noisy, that's why it has a ? next to the numbers in the table. (See the warning at the top of the page for more explanation.)

Furthermore, this change affects compiler front-end code so if there was a genuine regression for syn it should show up for syn-debug and syn-check benchmarks too.

@bors r=Centril

@bors
Copy link
Contributor

bors commented Feb 19, 2020

📌 Commit e761f3a has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2020
@Dylan-DPC-zz
Copy link

@bors p=1

@bors
Copy link
Contributor

bors commented Feb 20, 2020

⌛ Testing commit e761f3a with merge 183e893...

@bors
Copy link
Contributor

bors commented Feb 20, 2020

☀️ Test successful - checks-azure
Approved by: Centril
Pushing 183e893 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 20, 2020
@bors bors merged commit 183e893 into rust-lang:master Feb 20, 2020
@nnethercote nnethercote deleted the misc-inlining branch February 20, 2020 05:39
@jonas-schievink jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes. labels Mar 13, 2020
@jonas-schievink jonas-schievink modified the milestones: 1.44, 1.43 Mar 13, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 16, 2020
Pkgsrc changes:
 * Bump rust bootstrap version to 1.42.0, except for Darwin/i686 where the
   bootstrap is not (yet?) available.

Upstream changes:

Version 1.43.0 (2020-04-23)
==========================

Language
--------
- [Fixed using binary operations with `&{number}` (e.g. `&1.0`) not having
  the type inferred correctly.][68129]
- [Attributes such as `#[cfg()]` can now be used on `if` expressions.][69201]

**Syntax only changes**
- [Allow `type Foo: Ord` syntactically.][69361]
- [Fuse associated and extern items up to defaultness.][69194]
- [Syntactically allow `self` in all `fn` contexts.][68764]
- [Merge `fn` syntax + cleanup item parsing.][68728]
- [`item` macro fragments can be interpolated into `trait`s, `impl`s,
  and `extern` blocks.][69366]
  For example, you may now write:
  ```rust
  macro_rules! mac_trait {
      ($i:item) => {
          trait T { $i }
      }
  }
  mac_trait! {
      fn foo() {}
  }
  ```
These are still rejected *semantically*, so you will likely receive an error but
these changes can be seen and parsed by macros and
conditional compilation.


Compiler
--------
- [You can now pass multiple lint flags to rustc to override the previous
  flags.][67885] For example; `rustc -D unused -A unused-variables` denies
  everything in the `unused` lint group except `unused-variables` which
  is explicitly allowed. However, passing `rustc -A unused-variables -D unused` denies
  everything in the `unused` lint group **including** `unused-variables` since
  the allow flag is specified before the deny flag (and therefore overridden).
- [rustc will now prefer your system MinGW libraries over its bundled libraries
  if they are available on `windows-gnu`.][67429]
- [rustc now buffers errors/warnings printed in JSON.][69227]

Libraries
---------
- [`Arc<[T; N]>`, `Box<[T; N]>`, and `Rc<[T; N]>`, now implement
  `TryFrom<Arc<[T]>>`,`TryFrom<Box<[T]>>`, and `TryFrom<Rc<[T]>>`
  respectively.][69538] **Note** These conversions are only available when `N`
  is `0..=32`.
- [You can now use associated constants on floats and integers directly, rather
  than having to import the module.][68952] e.g. You can now write `u32::MAX` or
  `f32::NAN` with no imports.
- [`u8::is_ascii` is now `const`.][68984]
- [`String` now implements `AsMut<str>`.][68742]
- [Added the `primitive` module to `std` and `core`.][67637] This module
  reexports Rust's primitive types. This is mainly useful in macros
  where you want avoid these types being shadowed.
- [Relaxed some of the trait bounds on `HashMap` and `HashSet`.][67642]
- [`string::FromUtf8Error` now implements `Clone + Eq`.][68738]

Stabilized APIs
---------------
- [`Once::is_completed`]
- [`f32::LOG10_2`]
- [`f32::LOG2_10`]
- [`f64::LOG10_2`]
- [`f64::LOG2_10`]
- [`iter::once_with`]

Cargo
-----
- [You can now set config `[profile]`s in your `.cargo/config`, or through
  your environment.][cargo/7823]
- [Cargo will now set `CARGO_BIN_EXE_<name>` pointing to a binary's
  executable path when running integration tests or benchmarks.][cargo/7697]
  `<name>` is the name of your binary as-is e.g. If you wanted the executable
  path for a binary named `my-program`you would use
  `env!("CARGO_BIN_EXE_my-program")`.

Misc
----
- [Certain checks in the `const_err` lint were deemed unrelated to const
  evaluation][69185], and have been moved to the `unconditional_panic` and
  `arithmetic_overflow` lints.

Compatibility Notes
-------------------

- [Having trailing syntax in the `assert!` macro is now a hard error.][69548]
  This has been a warning since 1.36.0.
- [Fixed `Self` not having the correctly inferred type.][69340] This incorrectly
  led to some instances being accepted, and now correctly emits a hard error.

[69340]: rust-lang/rust#69340

Internal Only
-------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of `rustc` and
related tools.

- [All components are now built with `opt-level=3` instead of `2`.][67878]
- [Improved how rustc generates drop code.][67332]
- [Improved performance from `#[inline]`-ing certain hot functions.][69256]
- [traits: preallocate 2 Vecs of known initial size][69022]
- [Avoid exponential behaviour when relating types][68772]
- [Skip `Drop` terminators for enum variants without drop glue][68943]
- [Improve performance of coherence checks][68966]
- [Deduplicate types in the generator witness][68672]
- [Invert control in struct_lint_level.][68725]

[67332]: rust-lang/rust#67332
[67429]: rust-lang/rust#67429
[67637]: rust-lang/rust#67637
[67642]: rust-lang/rust#67642
[67878]: rust-lang/rust#67878
[67885]: rust-lang/rust#67885
[68129]: rust-lang/rust#68129
[68672]: rust-lang/rust#68672
[68725]: rust-lang/rust#68725
[68728]: rust-lang/rust#68728
[68738]: rust-lang/rust#68738
[68742]: rust-lang/rust#68742
[68764]: rust-lang/rust#68764
[68772]: rust-lang/rust#68772
[68943]: rust-lang/rust#68943
[68952]: rust-lang/rust#68952
[68966]: rust-lang/rust#68966
[68984]: rust-lang/rust#68984
[69022]: rust-lang/rust#69022
[69185]: rust-lang/rust#69185
[69194]: rust-lang/rust#69194
[69201]: rust-lang/rust#69201
[69227]: rust-lang/rust#69227
[69548]: rust-lang/rust#69548
[69256]: rust-lang/rust#69256
[69361]: rust-lang/rust#69361
[69366]: rust-lang/rust#69366
[69538]: rust-lang/rust#69538
[cargo/7823]: rust-lang/cargo#7823
[cargo/7697]: rust-lang/cargo#7697
[`Once::is_completed`]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed
[`f32::LOG10_2`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG10_2.html
[`f32::LOG2_10`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG2_10.html
[`f64::LOG10_2`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG10_2.html
[`f64::LOG2_10`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG2_10.html
[`iter::once_with`]: https://doc.rust-lang.org/std/iter/fn.once_with.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants